concourse/scripts/common.sh (1)

286-294: Consider adding error handling for the build and execution steps.

The function runs go build and executes the benchmark without checking for failures. If the build fails, the execution will also fail, potentially with confusing error messages.

🔎 Proposed fix with error handling

 run_bench_go() {
       pushd /home/$current_user/workspace/mongo/tests/gotests/bench_doc
       echo "build bench_doc"
-      go build
+      if ! go build; then
+            echo "Error: Failed to build bench_doc"
+            popd
+            return 1
+      fi
       echo "run bench_doc"
       ./bench_doc -eloqdoc-only
       $PREFIX/bin/eloqdoc-cli benchmark_db_11 --eval "db.dropDatabase()"
       popd
 }

concourse/scripts/main.ent.bash (1)

57-66: Consider adding cleanup between test phases for robustness.

The script launches and shuts down EloqDoc multiple times without cleanup between phases (lines 57-61 and 62-66). If shutdown_eloqdoc fails or leaves processes running, the subsequent launch_eloqdoc could encounter port conflicts or resource issues.

Consider adding a cleanup step between phases:

 launch_eloqdoc "$BUCKET_NAME" "$BUCKET_PREFIX"
 try_connect
 run_tpcc
 shutdown_eloqdoc
+cleanup_all "$DATA_DIR" "$BUCKET_NAME" "$BUCKET_PREFIX"

 launch_eloqdoc "$BUCKET_NAME" "$BUCKET_PREFIX"
 try_connect
 run_bench_go
 shutdown_eloqdoc
+cleanup_all "$DATA_DIR" "$BUCKET_NAME" "$BUCKET_PREFIX"

 launch_eloqdoc "$BUCKET_NAME" "$BUCKET_PREFIX"

This ensures a clean state between test phases and reduces the risk of interference.

tests/gotests/bench_doc/main.go (2)

26-39: Consider making connection URIs and database configuration externally configurable.

The URIs, database name, and other configuration constants are hardcoded. For a benchmarking tool that may need to run in different environments or configurations, consider using environment variables or command-line flags.

🔎 Example approach using environment variables with defaults

 // Configuration constants
 const (
-	mongoURI          = "mongodb://127.0.0.1:17017"
-	eloqdocURI        = "mongodb://127.0.0.1:27017"
-	dbName            = "benchmark_db_11"
 	singleInsertCount = 100
 	batchSize         = 100
 	numBatches        = 100
 	maxRetries        = 4
 	retryDelayMs      = 10000
 	loaderThreads     = 3
 	benchmarkDataDir  = "./benchmark_data"
 	deleteBatchSize   = 500
 )
+
+// Get configuration from environment or use defaults
+var (
+	mongoURI   = getEnv("MONGO_URI", "mongodb://127.0.0.1:17017")
+	eloqdocURI = getEnv("ELOQDOC_URI", "mongodb://127.0.0.1:27017")
+	dbName     = getEnv("DB_NAME", "benchmark_db_11")
+)
+
+func getEnv(key, defaultValue string) string {
+	if value := os.Getenv(key); value != "" {
+		return value
+	}
+	return defaultValue
+}

---

371-387: Review the exponential backoff implementation.

The retry logic uses exponential backoff with retryDelayMs*(1<<uint(attempt-1)). For retryDelayMs=10000 (10 seconds):

• Attempt 1: 10s delay
• Attempt 2: 20s delay
• Attempt 3: 40s delay
• Attempt 4: 80s delay

This totals 150 seconds of retry delays for 4 retries, which may be excessive for a benchmark tool where fast failure might be preferable.

Consider either:

1. Reducing retryDelayMs or maxRetries
2. Using a capped exponential backoff
3. Making retry behavior configurable for different operation types

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

tests/gotests/bench_doc/go.mod (1)

18-18: Update golang.org/x/crypto to address multiple critical security vulnerabilities.

This indirect dependency is affected by several critical CVEs including authorization bypass (CVE-2024-45337), denial of service via slow key exchanges (CVE-2025-22869), and multiple memory-related vulnerabilities in the SSH components. Upgrade to v0.35.0 or later.

#!/bin/bash
# Verify the latest safe version of golang.org/x/crypto
curl -s "https://proxy.golang.org/golang.org/x/crypto/@v/list" | tail -5

tests/gotests/bench_doc/main.go (1)

715-724: Query range calculations may produce unexpected values if document generation changes.

While current data generation ensures minimum Price of 10000 and Grams of 1000, these calculations lack explicit guards. If the data generation logic changes, RangeMinPrice, AndMinGrams, or ComplexMinPrice could become negative.

concourse/scripts/common.sh (1)

286-293: New benchmark function follows established patterns.

The function correctly builds and runs the Go benchmark tool with EloqDoc-only mode and cleans up the database afterward. The pushd/popd are balanced.

Note: The database name benchmark_db_11 is hardcoded here and in main.go (line 30). Consider extracting this to a shared constant or configuration to prevent drift.

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

tests/gotests/bench_doc/go.mod (1)

18-18: Update golang.org/x/crypto to address security vulnerabilities.

The dependency golang.org/x/crypto v0.26.0 is affected by multiple critical vulnerabilities including CVE-2024-45337 (authorization bypass), CVE-2025-22869 (denial of service), and several other CVEs disclosed in 2025. Upgrade to v0.35.0 or later to address these issues.

Run the following commands to update:

#!/bin/bash
cd tests/gotests/bench_doc
go get golang.org/x/crypto@v0.35.0
go mod tidy

tests/gotests/bench_doc/main.go (3)

203-209: Add validation for mutually exclusive flags.

The -mongo-only and -eloqdoc-only flags can both be set simultaneously, leading to unexpected behavior. Add validation after flag.Parse() to ensure they are mutually exclusive.

🔎 Proposed fix

 	outputFormat := flag.String("output", "table", "Output format: table, json, csv")
 	outputFile := flag.String("output-file", "", "Output file path (optional, defaults to stdout for table, or auto-generated filename)")
 	flag.Parse()
+
+	if *mongoOnly && *eloqdocOnly {
+		log.Fatal("Error: -mongo-only and -eloqdoc-only flags are mutually exclusive")
+	}

---

700-711: Guard against empty documents slice.

If document loading fails or returns no documents, rand.Intn(len(docs)) at lines 705-707 will panic. Add validation at the start of the function.

🔎 Proposed fix

 func generateQueryParams(docs []Cart, count int) []QueryParams {
+	if len(docs) == 0 {
+		log.Fatal("Error: No documents available for generating query parameters")
+	}
 	params := make([]QueryParams, count)

---

713-727: Prevent negative values in query ranges.

Lines 715, 718, and 723 calculate ranges by subtracting offsets, which can produce negative RangeMinPrice, AndMinGrams, or ComplexMinPrice if base values are small.

🔎 Proposed fix to clamp minimum values

+func max(a, b int) int {
+	if a > b {
+		return a
+	}
+	return b
+}
+
 func generateQueryParams(docs []Cart, count int) []QueryParams {
 	params := make([]QueryParams, count)
 
 	for i := 0; i < count; i++ {
 		// Select random documents for query parameters
 		doc1 := docs[rand.Intn(len(docs))]
 		doc2 := docs[rand.Intn(len(docs))]
 		doc3 := docs[rand.Intn(len(docs))]
 
 		item1 := doc1.Items[0]
 		item2 := doc2.Items[0]
 		item3 := doc3.Items[0]
 
 		params[i] = QueryParams{
 			ExactMatchID:     item1.ID,
-			RangeMinPrice:    item1.Price - 5000,
+			RangeMinPrice:    max(0, item1.Price - 5000),
 			RangeMaxPrice:    item1.Price + 5000,
 			AndQuantity:      item1.Quantity,
-			AndMinGrams:      item1.Grams - 1000,
+			AndMinGrams:      max(0, item1.Grams - 1000),
 			OrVariantID1:     item1.VariantID,
 			OrVariantID2:     item2.VariantID,
 			ArrayContainsID:  item1.ID,
 			NestedPrice:      item1.Price,
-			ComplexMinPrice:  item3.Price - 3000,
+			ComplexMinPrice:  max(0, item3.Price - 3000),
 			ComplexMaxGrams:  item3.Grams + 1000,
 			ComplexVariantID: item3.VariantID,
 		}
 	}

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 279
File: src/mongo/db/dbmain.cpp:0-0
Timestamp: 2025-11-14T03:10:22.027Z
Learning: In this repository, avoid defining --config via gflags in src/mongo/db/dbmain.cpp; MongoDB’s options_parser owns --config. Prefer gflags flags with a distinct prefix (e.g., --data_substrate_config) parsed before options_parser, with options_parser configured to allow unregistered options.

tests/gotests/bench_doc/go.mod (1)

18-18: Update golang.org/x/crypto to address critical security vulnerabilities.

The dependency golang.org/x/crypto v0.26.0 is affected by multiple critical vulnerabilities identified by OSV Scanner, including authorization bypass (CVE-2024-45337), denial of service (CVE-2025-22869), and unbounded memory consumption issues. Upgrade to at least v0.35.0.

cd tests/gotests/bench_doc && go get golang.org/x/crypto@latest && go mod tidy

concourse/scripts/common.sh (1)

286-294: Add error handling for go build and ./bench_doc execution.

The function lacks exit status checks after both commands. If compilation or the benchmark fails, the script will continue silently, potentially masking CI failures.

🔎 Proposed fix

 run_bench_go() {
       pushd /home/$current_user/workspace/mongo/tests/gotests/bench_doc
       echo "build bench_doc"
-      go build
+      go build || { echo "Error: Failed to build bench_doc"; popd; exit 1; }
       echo "run bench_doc"
-      ./bench_doc -eloqdoc-only
+      ./bench_doc -eloqdoc-only || { echo "Error: bench_doc execution failed"; popd; exit 1; }
       $PREFIX/bin/eloqdoc-cli benchmark_db_11 --eval "db.dropDatabase()"
       popd
 }

tests/gotests/bench_doc/main.go (3)

203-209: Add validation for mutually exclusive flags.

The -mongo-only and -eloqdoc-only flags can both be set simultaneously, leading to undefined behavior (MongoDB documents generated but EloqDoc folder used for query params).

🔎 Proposed fix

 	flag.Parse()
+
+	if *mongoOnly && *eloqdocOnly {
+		log.Fatal("Error: -mongo-only and -eloqdoc-only flags are mutually exclusive")
+	}

---

700-711: Add guard for empty docs slice to prevent panic.

If docs is empty (e.g., document loading fails), rand.Intn(len(docs)) will panic with "invalid argument to Intn".

🔎 Proposed fix

 func generateQueryParams(docs []Cart, count int) []QueryParams {
+	if len(docs) == 0 {
+		log.Fatal("Error: No documents available for generating query parameters")
+	}
 	params := make([]QueryParams, count)

---

713-726: Clamp range values to prevent negative query parameters.

RangeMinPrice (line 715) and AndMinGrams (line 718) can become negative if base values are small, potentially causing unexpected query behavior.

🔎 Proposed fix

 		params[i] = QueryParams{
 			ExactMatchID:     item1.ID,
-			RangeMinPrice:    item1.Price - 5000,
+			RangeMinPrice:    max(0, item1.Price - 5000),
 			RangeMaxPrice:    item1.Price + 5000,
 			AndQuantity:      item1.Quantity,
-			AndMinGrams:      item1.Grams - 1000,
+			AndMinGrams:      max(0, item1.Grams - 1000),
 			OrVariantID1:     item1.VariantID,
 			OrVariantID2:     item2.VariantID,
 			ArrayContainsID:  item1.ID,
 			NestedPrice:      item1.Price,
-			ComplexMinPrice:  item3.Price - 3000,
+			ComplexMinPrice:  max(0, item3.Price - 3000),
 			ComplexMaxGrams:  item3.Grams + 1000,
 			ComplexVariantID: item3.VariantID,
 		}

Note: Go 1.21+ has built-in max(). For earlier versions, define a helper function.

tests/gotests/bench_doc/main.go (2)

1462-1483: Use defer for client.Disconnect to ensure cleanup on all code paths.

If an error occurs between mongo.Connect and client.Disconnect, the connection may leak. Using defer ensures the connection is always closed.

🔎 Proposed fix

 		client, err := mongo.Connect(ctx, options.Client().ApplyURI(info.uri))
 		if err != nil {
 			log.Printf("Failed to connect for cleanup: %v", err)
 			continue
 		}
+		defer client.Disconnect(ctx)
 
 		coll := client.Database(info.dbName).Collection(info.collection)
 		err = retryOperation(func() error {
 			return coll.Drop(ctx)
 		}, maxRetries)
 		if err != nil {
 			log.Printf("Failed to drop collection %s after %d retries: %v", info.collection, maxRetries, err)
 		} else {
 			fmt.Printf("    ✓ Dropped collection: %s\n", info.collection)
 		}
-
-		client.Disconnect(ctx)
 	}

---

371-387: Consider adding jitter to exponential backoff.

The retry logic uses pure exponential backoff without jitter. For improved reliability in concurrent scenarios, consider adding randomized jitter to prevent synchronized retries.

🔎 Proposed enhancement (optional)

 		if attempt > 0 {
-			delay := time.Duration(retryDelayMs*(1<<uint(attempt-1))) * time.Millisecond
+			baseDelay := retryDelayMs * (1 << uint(attempt-1))
+			jitter := rand.Intn(baseDelay / 4) // 0-25% jitter
+			delay := time.Duration(baseDelay+jitter) * time.Millisecond
 			time.Sleep(delay)

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Learnt from: githubzilla
Repo: eloqdata/eloqdoc PR: 279
File: src/mongo/db/dbmain.cpp:0-0
Timestamp: 2025-11-14T03:10:22.027Z
Learning: In this repository, avoid defining --config via gflags in src/mongo/db/dbmain.cpp; MongoDB’s options_parser owns --config. Prefer gflags flags with a distinct prefix (e.g., --data_substrate_config) parsed before options_parser, with options_parser configured to allow unregistered options.